Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MIDI port listing #97

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

graysonguarino
Copy link

Adds an option -pl to list MIDI ports.

When testing this emulator, I was struggling to tell what my MIDI ports were as I have multiple MIDI devices plugged in, some with multiple ports. This helped me determine the one I wished to use.

@mckuhei
Copy link

mckuhei commented Sep 26, 2024

Did you forgot change midi_win32.cpp?

@graysonguarino
Copy link
Author

Did you forgot change midi_win32.cpp?

I sure did. I’ll get that revised when I get a chance.

@graysonguarino
Copy link
Author

I have not tested the previous commit as I'm struggling to build this on Windows (I've never done Windows C++ development before and cannot figure out how to install SDL2.)

Can you either test that for me, or give me some help on how to install SDL2?

@Gerwin2k
Copy link

I had a look at the windows code. There are some issues.

#include <windows.h> needs to be lowercase (pre-existing issue)

midiInGetID(midi_handle, puDeviceId);
there I changed a case error at ID, and removed the '&' twice in the arguments. Otherwise it won't compile.

printf("Port[%u]: %s\n", i, puDeviceId);
Here you are trying to print an integer puDeviceId as if it is a string.

@Gerwin2k
Copy link

@graysonguarino
Copy link
Author

I had a look at the windows code. There are some issues.

#include <windows.h> needs to be lowercase (pre-existing issue)

midiInGetID(midi_handle, puDeviceId); there I changed a case error at ID, and removed the '&' twice in the arguments. Otherwise it won't compile.

printf("Port[%u]: %s\n", i, puDeviceId); Here you are trying to print an integer puDeviceId as if it is a string.

Thank you for looking into this! That means that I didn’t do what I had intended. I’m trying to print a named descriptor of the MIDI device (see the rtmidi implementation in this PR) to print out. All of Microsoft’s types are throwing me for a loop as someone unfamiliar with the win32 API.

@graysonguarino
Copy link
Author

It seems like to get a device name, I should have used this: https://learn.microsoft.com/en-us/windows/win32/api/mmeapi/nf-mmeapi-midiingetdevcaps

@Gerwin2k can you give me a tip on how to install SDL2? I’m using Visual Studio 2022 to build because I don’t know anything about the C++ build process on Windows (I’m only familiar with *nix systems). If you can point me to a better resource, I can use that instead.

@Gerwin2k
Copy link

I don't use any recent MSVC. Instead I use MinGW with Code::Blocks IDE.
if you do a web search "sdl2 msvc 2022" there are several setup guides.

Note that Nuked-SC55 already reports the Midi port description string in the console, but only for the single port that it is using.

@graysonguarino
Copy link
Author

Note that Nuked-SC55 already reports the Midi port description string in the console, but only for the single port that it is using.

I just committed a fix for my previous commit that copies the behavior of what you described. I'm still struggling to get MinGW running and won't have time to check this tonight, but since it's a copy/paste job, I imagine there shouldn't be an issue.

If nobody is free to check this today, I can get back to creating a Windows build environment tomorrow to check my work.

Thank you again for all your help!

@Gerwin2k
Copy link

With your recent change it works well. See attached screenshot.
listmidi

@graysonguarino
Copy link
Author

With your recent change it works well. See attached screenshot.

That’s the behavior I expect! Thanks again for all of your help.

@Kappa971
Copy link

Kappa971 commented Oct 1, 2024

@graysonguarino a guide to compiling on Windows with Visual Studio: #44 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants